require explicitly specifying node name in subscription pubsub #607
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
modify pubsub behaviour to include `node_name/0` callback
|
Hey @azhi I appreciate the PR! We're gonna be reconfiguring the whole internals of PubSub for 1.5 probably, so I my hold off on merging this for a while so I can evaluate it in that context. |
|
Oh, ok, I guess I'll have to revisit all this stuff when upgrading to 1.5 :). I was also gonna make some work in supporting Direct usage of phoenix pubsub in absinthe_phoenix. Do you want me to open PR for that, or this part will also be reworked in 1.5? |
|
Thanks! |
|
@azhi by all means do a PR for direct usage. I'm hoping we wouldn't have to change very much but please go for it! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
modify pubsub behaviour to include
node_name/0callbackJustification for breaking change
Currently pubsub system (specifically pubsub proxy server) relies on
Kernel.node/0to check whether message is local or not.Kernel.nodeis a function that was intended to work inside erlang cluster.But pubsub system can be run in a "cluster" connected through a bunch of mechanisms, not only in erlang cluster. Phoenix pubsub recognizes this and includes a node_name function in it's specification, which is used in different pubsub adapters, such as redis one, where you could explicitly specify node name.
In fact, this is the background for this PR - I was using redis adapter, and couldn't make subscriptions work. Turns out it was because of this node names issue. It could be fixed by switching from using explicit
node_nameinPhoenix.PubSub.Redisconfig to using--snameflag, but I think it is worth fixing now when not a lot of people is dependent on currentAbsinthe.Subscription.Pubsub.